Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(direct-access): save context not present if direct access enabled #6764

Merged
merged 6 commits into from
Jul 17, 2020

Conversation

omairvaiyani
Copy link
Contributor

@omairvaiyani omairvaiyani commented Jun 30, 2020

Related PR introduced a new feature but does not work with experimental Direct Access which uses ParseServerRESTController.

Proposed change just adds context into data as expected by RestWrite

Open discussion for feature with other issues

@codecov
Copy link

codecov bot commented Jun 30, 2020

Codecov Report

Merging #6764 into master will decrease coverage by 0.58%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6764      +/-   ##
==========================================
- Coverage   93.90%   93.32%   -0.59%     
==========================================
  Files         169      169              
  Lines       12197    12197              
==========================================
- Hits        11454    11383      -71     
- Misses        743      814      +71     
Impacted Files Coverage Δ
...dapters/Cache/RedisCacheAdapter/KeyPromiseQueue.js 0.00% <0.00%> (-95.46%) ⬇️
src/Adapters/Cache/RedisCacheAdapter/index.js 12.50% <0.00%> (-82.15%) ⬇️
src/Routers/PushRouter.js 93.10% <0.00%> (-3.45%) ⬇️
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 92.87% <0.00%> (-0.67%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f7fd17...c0508e9. Read the comment docs.

@dplewis
Copy link
Member

dplewis commented Jun 30, 2020

Can you add a test case? Looks good!

@dplewis
Copy link
Member

dplewis commented Jul 13, 2020

@yog27ray Shouldn't this be fix via #6666 now that you moved context to the middleware?

@dplewis
Copy link
Member

dplewis commented Jul 17, 2020

@omairvaiyani I'm going to add a test case to cover direct access. This has already been fixed via
https://github.com/parse-community/parse-server/pull/6666/files#diff-698500347c579cf381cfce33fb74e2ff

@dplewis dplewis merged commit 85ec22c into master Jul 17, 2020
@dplewis dplewis deleted the patch/save-context-with-direct-access branch July 17, 2020 03:56
@omairvaiyani
Copy link
Contributor Author

@dplewis thanks for that! My apologies for the uncertainty around the fix. We've been settled on parse-server v3~ since the GraphQL Relay change so my patches are hazy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants